-
Notifications
You must be signed in to change notification settings - Fork 770
🔄 refactor(model): memory usage optimisation #2813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
🔄 refactor(model): memory usage optimisation #2813
Conversation
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
…emorybank-memory_gpu_usage
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
…emory bank framework Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
…ttps://github.com/alfieroddan/anomalib into refactor/model/optimise-memorybank-memory_gpu_usage
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
…u_usage Signed-off-by: alfieroddan <51797647+alfieroddan@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work.
I have some small comments.
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great, thanks for the refactor!
Only have few minor comments.
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
…ian. Test for None replacements for args Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
Signed-off-by: Alfie Roddan <51797647+alfieroddan@users.noreply.github.com>
self.memory_bank = torch.empty(0) | ||
|
||
@deprecate(args={"dataset": None}, since="2.1.0") | ||
def fit(self, dataset: torch.Tensor = None) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we would completely remove the dataset param.
Not many uses/knows about it
📝 Description
The memory bank models currently keep training embeddings, model's stored embeddings on device instead of releasing.
This can be seen most dramatically in Patchcore and PaDiM. We expect peak memory to drop in validation but actually we keep the training embeddings in memory:
As an example here is Patchcore's lightning fit function, and I've commented where we should delete duplicate embeddings:
In addition to duplicate embeddings we should also be storing embeddings on the model not in lightning. So that the model can be used stand alone.
Lastly, the memory bank models are not designed for DDP or multi-gpu use. I have put default args to fix this.
✨ Changes
Select what type of change your PR is:
✅ Checklist
Before you submit your pull request, please make sure you have completed the following steps:
For more information about code review checklists, see the Code Review Checklist.